Skip to content

fix(docker): enable docker logs for pd/store/server containers#2980

Open
bitflicker64 wants to merge 8 commits intoapache:masterfrom
bitflicker64:fix-docker-logs
Open

fix(docker): enable docker logs for pd/store/server containers#2980
bitflicker64 wants to merge 8 commits intoapache:masterfrom
bitflicker64:fix-docker-logs

Conversation

@bitflicker64
Copy link
Copy Markdown
Contributor

@bitflicker64 bitflicker64 commented Mar 28, 2026 •
edited
Loading

Purpose of the PR

docker logs shows no application output for any HugeGraph container. All JVM logs are silently redirected to files inside the container, requiring manual exec + tail to debug. This PR fixes that.

related comment - #2952 (comment)

Main Changes

Two root causes fixed:

  1. Startup scripts redirected JVM stdout/stderr to a file via >> ${OUTPUT} 2>&1 before Docker could capture it — now gated behind a STDOUT_MODE=true env var so bare metal installs are unaffected
  2. The console appender was defined in all dist log4j2.xml configs but never wired to the root logger and org.apache.hugegraph AsyncLogger — added <appender-ref ref="console"/> to both in all 3 configs

Docker compose files set STDOUT_MODE=true so containers stream logs via docker logs. File appenders are kept so non-Docker deployments are unaffected.

Also sets STDOUT_MODE=true as a default in Dockerfiles to ensure docker logs works for direct docker run usage, not just docker compose.

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)

Does this PR potentially affect the following parts?

  • Dependencies
  • Modify configurations
  • The public API
  • Other affects (typed here)
  • Nope

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

@imbajin imbajin requested a review from Copilot March 28, 2026 13:07
Copilot AI reviewed Mar 28, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to make HugeGraph PD/Store/Server JVM logs visible via docker logs by stopping shell-level stdout/stderr redirection and ensuring Log4j2 emits to the console.

Changes:

  • Removed >> ... 2>&1 redirections from startup scripts so Docker can capture stdout/stderr.
  • Added console appender references to the Log4j2 root logger in dist configs.
  • Kept file appenders so non-Docker deployments still write log files.

Reviewed changes

Copilot reviewed 3 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
hugegraph-store/hg-store-dist/src/assembly/static/conf/log4j2.xml Adds console appender to root logger.
hugegraph-store/hg-store-dist/src/assembly/static/bin/start-hugegraph-store.sh Stops redirecting JVM output to a file.
hugegraph-server/hugegraph-dist/src/assembly/static/conf/log4j2.xml Adds console appender to root logger (but see comment re AsyncLogger wiring).
hugegraph-server/hugegraph-dist/src/assembly/static/bin/start-hugegraph.sh Stops redirecting daemon/foreground output to a file (but see comment re foreground PID handling).
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh Stops redirecting JVM output to a file.
hugegraph-pd/hg-pd-dist/src/assembly/static/conf/log4j2.xml Adds console appender to root logger.
hugegraph-pd/hg-pd-dist/src/assembly/static/bin/start-hugegraph-pd.sh Stops redirecting JVM output to a file.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bitflicker64 bitflicker64 requested a review from Copilot March 28, 2026 13:16
Copilot AI reviewed Mar 28, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 7 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bitflicker64
Copy link
Copy Markdown
Contributor Author

bitflicker64 commented Mar 28, 2026

@imbajin I've wired the console appender into org.apache.hugegraph across all three components. Should I also add it to the third-party loggers or is just the HugeGraph application logger sufficient for the Docker logging goal?

vaijosh
vaijosh approved these changes Mar 28, 2026
View reviewed changes
Copy link
Copy Markdown

@vaijosh vaijosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Mar 28, 2026
@imbajin
Copy link
Copy Markdown
Member

imbajin commented Apr 1, 2026

@imbajin I've wired the console appender into org.apache.hugegraph across all three components. Should I also add it to the third-party loggers or is just the HugeGraph application logger sufficient for the Docker logging goal?

Seems not necessary for now

@bitflicker64
Copy link
Copy Markdown
Contributor Author

bitflicker64 commented Apr 2, 2026

@imbajin On it fixing both the additivity loggers and the shell redirect gate. Will push shortly.

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Apr 2, 2026
@bitflicker64 bitflicker64 requested a review from imbajin April 2, 2026 19:51
@imbajin imbajin requested a review from Copilot April 3, 2026 10:24

This comment was marked as duplicate.

Sign in to view

imbajin
imbajin reviewed Apr 3, 2026
View reviewed changes
@bitflicker64
Copy link
Copy Markdown
Contributor Author

bitflicker64 commented Apr 3, 2026

STDOUT_MODE is currently only set via compose, so direct docker run won’t expose logs via docker logs

@bitflicker64 bitflicker64 requested a review from Copilot April 3, 2026 17:22
Copilot AI reviewed Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 13 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bitflicker64 bitflicker64 requested a review from Copilot April 3, 2026 17:29
Copilot AI reviewed Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 13 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bitflicker64 bitflicker64 requested a review from imbajin April 3, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@imbajin imbajin Awaiting requested review from imbajin

+1 more reviewer

@vaijosh vaijosh vaijosh approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix(docker): enable docker logs for pd/store/server containers

4 participants

@bitflicker64 @imbajin @vaijosh